Skip to content

feat: adding mandatory flag to string input #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

feat: adding mandatory flag to string input #844

wants to merge 3 commits into from

Conversation

@Nico-DF
Copy link
Contributor

@Nico-DF Nico-DF commented Aug 4, 2023

As I began to use spring-shell for some CLI workflow, I found that there is no input required for string (altough there is default).

For the moment, I used this kind of code:

public String input(String message, String defaultValue, boolean mandatory) {
	var component = new StringInput(getTerminal(), message, defaultValue);
	component.setResourceLoader(getResourceLoader());
	component.setTemplateExecutor(getTemplateExecutor());
	var context = component.run(ConfirmationInput.ConfirmationInputContext.empty());
	var result = context.getInput();

	while (mandatory && !StringUtils.hasText(result)) {
		result = component.run(ConfirmationInput.ConfirmationInputContext.empty()).getInput();
	}

	return result;
}

or this as flow:

componentFlowBuilder.clone().reset() //
    .withStringInput("id")
	.name("Select an identifier")
	.next(ctx -> StringUtils.isBlank(ctx.getInput()) ? "id" : null)
    .and().build();

Which work but it could be simpler, hence my PR.

So I:

  • Added to flow the mandatory flag
  • Added to component the mandatory flag
  • Add simple test case
  • Add samples

Result:
image
image
image

@Nico-DF Nico-DF force-pushed the feature/string-required branch 2 times, most recently from 886336a to 93cd27f Compare August 4, 2023 14:24
@Nico-DF
Copy link
Contributor Author

Nico-DF commented Aug 8, 2023 •
edited
Loading

Issue with jdk8 compilation doesn't seem to originate from me, right?

(investigating though to be sure)

@Nico-DF
Copy link
Contributor Author

Nico-DF commented Aug 8, 2023

As suspected I broke something :D, I'll check it

@jvalkeal
Copy link
Contributor

jvalkeal commented Aug 8, 2023

I'm just thinking if this is going to be a bit confusing for a user if there's a default value.

my-shell:>component string required
? Enter value [Required][Default myvalue]

I'm kinda questioning if there's better way to give indication to user that something is required.

@Nico-DF
Copy link
Contributor Author

Nico-DF commented Aug 8, 2023

I'm just thinking if this is going to be a bit confusing for a user if there's a default value.

my-shell:>component string required
? Enter value [Required][Default myvalue]

I'm kinda questioning if there's better way to give indication to user that something is required.

I also wondered about it. I think the easiest way would be to display Required only if there is no default (and it also matches with what really happens)

@fmbenhassine
Copy link
Contributor

fmbenhassine commented Oct 30, 2025

@Nico-DF Can you please rebase this PR on the latest main? I will check if we can include it in the upcoming 4.0. Thank you upfront.

@fmbenhassine fmbenhassine added the status/need-feedback Calling participant to provide feedback label Oct 30, 2025
@Nico-DF Nico-DF force-pushed the feature/string-required branch from 28e5d29 to 903d2cf Compare October 30, 2025 11:41
@Nico-DF Nico-DF changed the base branch from 2.1.x to main October 30, 2025 11:41
@Nico-DF Nico-DF force-pushed the feature/string-required branch 2 times, most recently from 6305112 to cfd1619 Compare October 30, 2025 12:20
@Nico-DF
Copy link
Contributor Author

Nico-DF commented Oct 30, 2025

Update and signing done

@fmbenhassine
Copy link
Contributor

fmbenhassine commented Oct 30, 2025

Thank you for the quick updates! You were faster than I expected 😂 I was in the middle of a big refactoring when I asked for a rebase and I didn't think your update would come the same day..

Anyway, I will try to rebase your PR myself, otherwise I will let you know.

@Nico-DF
Copy link
Contributor Author

Nico-DF commented Oct 30, 2025

No problem, was in between tasks at work, so I had time to do it. I can rebase it tomorrow morning if you want, just ping me when all is finished on main

@fmbenhassine
Copy link
Contributor

fmbenhassine commented Oct 31, 2025

Thank you very much! But no rush, we can merge this (as well as #849) in RC1 next week. I will drop a message here when main is stable again (some restructuring is ongoing).

@fmbenhassine fmbenhassine added this to the 4.0.0 milestone Oct 31, 2025
@fmbenhassine
Copy link
Contributor

fmbenhassine commented Nov 27, 2025 •
edited
Loading

Hi @Nico-DF

I think we can proceed and merge this PR now. Can you please rebase it on the latest main?

Please exclude the changes in samples, they are outdated.

Thank you upfront.

@Nico-DF
feat: adding mandatory flag to string input
8a2d042 
- Added to flow
- Added to component
- Add simple test case
- Add samples

Signed-off-by: Nico-DF <[email protected]>
@Nico-DF
feat: rename mandatory to required
34d6a46 
Signed-off-by: Nico-DF <[email protected]>
@Nico-DF
fix: display required only if no default value
db745c6 
Signed-off-by: Nico-DF <[email protected]>
@Nico-DF Nico-DF force-pushed the feature/string-required branch from cfd1619 to db745c6 Compare November 27, 2025 14:54
@Nico-DF
Copy link
Contributor Author

Nico-DF commented Nov 27, 2025

Done 👍
Not had time to compile locally yet 😋

fmbenhassine pushed a commit that referenced this pull request Nov 28, 2025
@Nico-DF @fmbenhassine
Add mandatory flag to string input
2bdf843 
- Added to flow
- Added to component
- Add simple test case
- Add samples

PR #844

Signed-off-by: Nico-DF <[email protected]>
@fmbenhassine
Copy link
Contributor

fmbenhassine commented Nov 28, 2025

Thank you for the updates!

Not had time to compile locally yet 😋

I tired locally and it compiles! Ship it 😂 2bdf843

Thank you for your contribution!

@fmbenhassine fmbenhassine added type/enhancement Is an enhancement request and removed status/need-feedback Calling participant to provide feedback labels Nov 28, 2025
@fmbenhassine fmbenhassine added the area/components Belongs ui components label Nov 28, 2025
@fmbenhassine fmbenhassine modified the milestones: 4.0.0, 4.0.0-M2 Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jvalkeal jvalkeal jvalkeal left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/components Belongs ui components type/enhancement Is an enhancement request

Projects

None yet

Milestone

4.0.0-M2

Development

Successfully merging this pull request may close these issues.

3 participants

@Nico-DF @jvalkeal @fmbenhassine